Skip to content

PwmPin: allow get_duty to return the old value for some time#236

Merged
bors[bot] merged 1 commit into
rust-embedded:masterfrom
eldruin:pwm-get-old-ok
Jul 22, 2020
Merged

PwmPin: allow get_duty to return the old value for some time#236
bors[bot] merged 1 commit into
rust-embedded:masterfrom
eldruin:pwm-get-old-ok

Conversation

@eldruin

@eldruin eldruin commented Jul 21, 2020

Copy link
Copy Markdown
Member

This is #140 rebased to master.
However, I suppose the same thing applies to the Pwm::try_get_duty() method. Shouldn't the same note be added to that method as well?

Closes #140

PWM is often implemented in a buffered way to allow glitch-free
operation; as a result, implementing a strict "you get what you last
set" is not feasible for some implementations.
@rust-highfive

Copy link
Copy Markdown

r? @ryankurte

(rust_highfive has picked a reviewer for you, use r? to override)

@therealprof therealprof left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

bors r+

@therealprof

Copy link
Copy Markdown
Contributor

However, I suppose the same thing applies to the Pwm::try_get_duty() method. Shouldn't the same note be added to that method as well?

Sure, why not. The more documentation the better. ;)

@bors

bors Bot commented Jul 22, 2020

Copy link
Copy Markdown
Contributor

@bors bors Bot merged commit 5b898bb into rust-embedded:master Jul 22, 2020
bors Bot added a commit that referenced this pull request Jul 22, 2020
238:  Pwm: allow get_duty to return the old value for some time r=therealprof a=eldruin

This is #140 applied to the `Pwm` trait as well, since the same concerns apply. Follows up on #236.

Co-authored-by: Diego Barrios Romero <eldruin@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-review Review is incomplete T-hal

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants